Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Strategy to inject dependencies via class- and instance-methods #70

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

maxhollmann
Copy link

Injects dependencies using methods on the class and instance:

module Test
  AutoInject = Dry::AutoInject(some_dep: 'The dependency').methods
end

class SomeClass
  include Test::AutoInject[:some_dep]
end

SomeClass.some_dep
# => "The dependency"

SomeClass.new.some_dep
# => "The dependency"

with_other_dep = SomeClass.with_deps(some_dep: 'Other')
with_other_dep.some_dep
# => "Other"

with_other_dep.new.some_dep
# => "Other"

This leaves the constructor free to receive other arguments, which makes integrating this library into existing projects much easier.

@maxhollmann maxhollmann marked this pull request as draft November 20, 2020 12:30
@maxhollmann maxhollmann marked this pull request as ready for review November 20, 2020 13:01
@maxhollmann
Copy link
Author

Not sure why that kwargs spec fails, it passes locally...

@solnic
Copy link
Member

solnic commented Nov 23, 2020

Hey, thanks for the PR. In the future, it's better to first propose a new feature on our discussion forum. I'm not sure if this can be accepted so first I'd appreciate if you could expand on the idea that this would help integrating auto_inject with existing projects. I just need to be sure that in some cases this would indeed be helpful and using existing strategies wouldn't be an option.

@candland
Copy link

Found this while trying to use dry-rails with ActiveJob. Using this strategy would allow injection for jobs. I'm using the Container directly for now. ActiveJob tries to serialize all constructor args, which most dependencies won't serialize and would probably not be good even if they did.

+1 for this strategy

@solnic
Copy link
Member

solnic commented Dec 8, 2020

@candland that's a really good use case. Thanks. I think we could include it then. WDYT @timriley ?

@flash-gordon
Copy link
Member

you should all just use effects and call it a day 😆

@solnic
Copy link
Member

solnic commented Dec 8, 2020

@flash-gordon oh right, I'm much more inclined to switch to effects altogether at some point (sooner than later for sure), but for now, this sounds reasonable, no?

@flash-gordon
Copy link
Member

@solnic I don't have preferences over injection strategies. In this case, it looks strange that in order to inject a dep you'll get an anonymous copy of a class. I might have unwanted side effects but I guess it's up to the user to deal with it.

@candland
Copy link

candland commented Dec 8, 2020

@flash-gordon what are effects? Where I can read more about them?

@flash-gordon
Copy link
Member

@candland https://dry-rb.org/gems/dry-effects/0.1/effects/resolve/
I also have a sample project with dry-everything https://github.com/flash-gordon/rt-tracker
And there's an example of using effects in an existing rails project https://github.com/flash-gordon/forem

@solnic
Copy link
Member

solnic commented Dec 9, 2020

@maxhollmann woud it be possible for you to solve your problem using an effect-based injector like @flash-gordon suggests?

@maxhollmann
Copy link
Author

That might work, I'll check in the coming days. I share flash-gordon's concern with the anonymous subclass, so I'd be happy if I can get around it using the effects injector.

@solnic
Copy link
Member

solnic commented Dec 9, 2020

@maxhollmann cool! I think it's worth mentioning that we'll probably switch to an effect-based injector as the default way (maybe even the only officially supported way) at some point in the future, so testing this out would be very helpful 🙂

@maxhollmann
Copy link
Author

Okay, I finally got around to looking at the effects provider. It would indeed be much cleaner than this solution, but I haven't found a good way to provide the default dependencies everywhere in a Rails app (server, tests, console, rake/thor tasks). Does anyone know of a clean way to do this?

@flash-gordon
Copy link
Member

@maxhollmann there's no out-of-the-box solution I would say, I'm still gathering use cases to wrap them with a nice API. Normally, I create a single effect stack and re-use it in places like CLI or background tasks.
This is how it's typically done: https://github.com/flash-gordon/rt-tracker/blob/main/lib/rt_tracker/handlers_stack.rb
This is how it's used: https://github.com/flash-gordon/rt-tracker/blob/825855e07d135ed73547a541e2dbd73c6408956c/Rakefile#L13-L23
Really, nothing fancy.

@solnic
Copy link
Member

solnic commented Jan 13, 2021

Really, nothing fancy.

EXACTLY, this is what makes this so freaking awesome 🙂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants